Support configuration reloads for logging #6720

bevacqua · 2016-03-31T20:56:52Z

A fix for #4407. Users can reload configuration through the following command even if --no-watch is turned on.

kill -s SIGHUP `cat ./pid.log` # or wherever/whatever your pid may be

The even-better package is a fork of good that supports reloading configuration.

bevacqua · 2016-03-31T21:00:37Z

It should be noted that several different approaches were considered, and this was by far the simplest. For completeness:

Considered going this route but initially passed on it
Considered turning app into a cluster (master/ couple workers, then the master gets SIGHUP and recycles workers in sliding window), didn't try this route
Attempted a couple of ideas from @spalger where we'd proxy the hapi server in a number of places, but this proved to be very tricky to implement and it touched in lots of places, so I abandoned it
Went back to first option, which is this PR

spalger · 2016-04-01T02:38:51Z

package.json

@@ -97,6 +97,7 @@
    "d3": "3.5.6",
    "elasticsearch": "10.1.2",
    "elasticsearch-browser": "10.1.2",
+    "even-better": "7.0.0",


spalger · 2016-04-01T02:44:47Z

made some notes, but looks good otherwise!

bevacqua · 2016-04-01T17:25:24Z

@spalger Are you still reviewing this or should I assign somebody else now that you've betrayed us?

spalger · 2016-04-01T21:22:31Z

@bevacqua haha, yes.

spalger · 2016-04-05T14:34:44Z

Implementation looks great. Can you get write a couple high-level tests for this? I'm thinking it should probably use child processes which actually write to files and rotate from one file to another.

bevacqua · 2016-04-05T21:27:01Z

Added a test case for this. Let me know if there's an alternative that's better than waiting for a fixed amount of seconds.

spalger · 2016-04-05T23:31:35Z

src/cli/serve/__tests__/fixtures/reload_logging_config/kibana.test.yml

+server:
+  port: 8274
+logging:
+  dest: src/cli/serve/__tests__/logs/first.log


if you set the cwd of the process you can move this into code (where it will be easier to keep up to date)

Wat? Not sure how the cwd changes this at all, but I'm removing the log to file config in favor of what you mention in the next comment

spalger · 2016-04-05T23:50:48Z

jenkins, test it

bevacqua · 2016-04-06T22:47:12Z

@spalger Wrote a more precise test case

bevacqua · 2016-05-24T18:42:58Z

@spalger Test is now fixed

spalger · 2016-05-24T22:49:26Z

LGTM

bevacqua · 2016-05-25T00:45:10Z

@rashidkpc Maybe you want to give it the second pass?

w33ble · 2016-05-26T22:09:49Z

This looks good, but I hit an issue that I've previously only seen in dev mode. Usually, if you save changes often, you get a warning in node about a possible EventEmitter leak; I generally ignore this as I figured it was caused by the watch process.

However, I'm seeing the same message here after I issue several SIGHUP's to the server.

From the stack trace it looks like it's coming from this PR, as it starts from process.on('SIGHUP'...

Repro: PID=1234; while [ 1 ]; do kill -s SIGHUP $PID; sleep 2; done, you'll see the error the 10th time it's called.

Maybe it's actually a bug in even-better?

(node) warning: possible EventEmitter memory leak detected. 11 request-internal listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at [object Object].addListener (events.js:239:17)
    at [object Object].internals.Kilt.on.internals.Kilt.addListener (/Users/d33t/repos/kibana/node_modules/hapi/node_modules/kilt/lib/index.js:66:45)
    at new module.exports.Monitor.internals.NetworkMonitor (/Users/d33t/repos/kibana/node_modules/even-better/lib/network.js:18:18)
    at setupOpsMonitoring (/Users/d33t/repos/kibana/node_modules/even-better/lib/monitor.js:89:23)
    at /Users/d33t/repos/kibana/node_modules/even-better/lib/monitor.js:156:47
    at done (/Users/d33t/repos/kibana/node_modules/even-better/node_modules/items/lib/index.js:30:25)
    at KbnLogger.init (/Users/d33t/repos/kibana/src/server/logging/log_reporter.js:32:5)
    at /Users/d33t/repos/kibana/node_modules/even-better/lib/monitor.js:149:18
    at iterate (/Users/d33t/repos/kibana/node_modules/even-better/node_modules/items/lib/index.js:35:13)
    at Object.exports.serial (/Users/d33t/repos/kibana/node_modules/even-better/node_modules/items/lib/index.js:38:9)
    at [object Object].internals.Monitor.start (/Users/d33t/repos/kibana/node_modules/even-better/lib/monitor.js:124:11)
    at [object Object].reconfigure (/Users/d33t/repos/kibana/node_modules/even-better/lib/index.js:20:17)
    at KbnServer.applyLoggingConfiguration (/Users/d33t/repos/kibana/src/server/kbn_server.js:122:48)
    at process.reloadConfig (/Users/d33t/repos/kibana/src/cli/serve/serve.js:160:17)
    at emitNone (events.js:67:13)
    at process.emit (events.js:166:7)
    at Signal.wrap.onsignal (node.js:804:46)
(node) warning: possible EventEmitter memory leak detected. 11 response listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at [object Object].addListener (events.js:239:17)
    at [object Object].internals.Kilt.on.internals.Kilt.addListener (/Users/d33t/repos/kibana/node_modules/hapi/node_modules/kilt/lib/index.js:66:45)
    at new module.exports.Monitor.internals.NetworkMonitor (/Users/d33t/repos/kibana/node_modules/even-better/lib/network.js:19:18)
    at setupOpsMonitoring (/Users/d33t/repos/kibana/node_modules/even-better/lib/monitor.js:89:23)
    at /Users/d33t/repos/kibana/node_modules/even-better/lib/monitor.js:156:47
    at done (/Users/d33t/repos/kibana/node_modules/even-better/node_modules/items/lib/index.js:30:25)
    at KbnLogger.init (/Users/d33t/repos/kibana/src/server/logging/log_reporter.js:32:5)
    at /Users/d33t/repos/kibana/node_modules/even-better/lib/monitor.js:149:18
    at iterate (/Users/d33t/repos/kibana/node_modules/even-better/node_modules/items/lib/index.js:35:13)
    at Object.exports.serial (/Users/d33t/repos/kibana/node_modules/even-better/node_modules/items/lib/index.js:38:9)
    at [object Object].internals.Monitor.start (/Users/d33t/repos/kibana/node_modules/even-better/lib/monitor.js:124:11)
    at [object Object].reconfigure (/Users/d33t/repos/kibana/node_modules/even-better/lib/index.js:20:17)
    at KbnServer.applyLoggingConfiguration (/Users/d33t/repos/kibana/src/server/kbn_server.js:122:48)
    at process.reloadConfig (/Users/d33t/repos/kibana/src/cli/serve/serve.js:160:17)
    at emitNone (events.js:67:13)
    at process.emit (events.js:166:7)
    at Signal.wrap.onsignal (node.js:804:46)

bevacqua · 2016-06-01T20:27:18Z

@w33ble Fixed memory leak issues via [email protected] 1a1f679c

bevacqua · 2016-06-01T20:37:30Z

jenkins, test it

bevacqua · 2016-06-01T20:55:25Z

Passes 🎉 Re-running just in case

jenkins, test it

w33ble · 2016-06-01T22:02:28Z

LGTM!

bevacqua assigned spalger Mar 31, 2016

spalger reviewed Apr 1, 2016
View reviewed changes

spalger assigned bevacqua and unassigned spalger Apr 1, 2016

bevacqua force-pushed the feature/bouncy-castle branch from 696e459 to 27cb919 Compare April 1, 2016 17:22

[logging] Support configuration reloads for logging. Fixes elastic#4407.

b7dbfb9

bevacqua force-pushed the feature/bouncy-castle branch from 27cb919 to b7dbfb9 Compare April 1, 2016 17:23

bevacqua assigned spalger and unassigned bevacqua Apr 1, 2016

spalger added the tests_needed label Apr 5, 2016

spalger assigned bevacqua and unassigned spalger Apr 5, 2016

[test] Add tests for SIGHUP logging configuration reload signal.

4759daa

bevacqua assigned spalger and unassigned bevacqua Apr 5, 2016

bevacqua added review and removed tests_needed labels Apr 5, 2016

spalger reviewed Apr 5, 2016
View reviewed changes

[test/fix] Improved test case for logging configuration reloads.

f09b374

bevacqua assigned spalger and unassigned bevacqua May 24, 2016

bevacqua added the v5.0.0 label May 24, 2016

spalger assigned bevacqua and unassigned spalger May 24, 2016

w33ble assigned w33ble and unassigned bevacqua May 26, 2016

w33ble added the updates_needed label May 26, 2016

w33ble assigned bevacqua and unassigned w33ble May 26, 2016

[dep] Bump even-better to 7.0.2 fixing potential memory leak issues.

59981ba

bevacqua removed the updates_needed label Jun 1, 2016

bevacqua assigned w33ble and unassigned bevacqua Jun 1, 2016

bevacqua merged commit ef1fb42 into elastic:master Jun 1, 2016

bevacqua unassigned w33ble Jun 1, 2016

epixa added the v5.0.0-alpha4 label Jun 17, 2016

epixa added v5.0.0 and removed v5.0.0 labels Jun 28, 2016

This was referenced Nov 8, 2016

Document configuration reloading #9008

Closed

Allow dynamic configuration of logging #9007

Closed

ppf2 mentioned this pull request Jan 24, 2018

Perform deprecated settings transformation as part of config reloading #16245

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support configuration reloads for logging #6720

Support configuration reloads for logging #6720

bevacqua commented Mar 31, 2016

bevacqua commented Mar 31, 2016

spalger Apr 1, 2016

spalger commented Apr 1, 2016

bevacqua commented Apr 1, 2016

spalger commented Apr 1, 2016

spalger commented Apr 5, 2016

bevacqua commented Apr 5, 2016

spalger Apr 5, 2016

bevacqua Apr 6, 2016

spalger commented Apr 5, 2016

bevacqua commented Apr 6, 2016

bevacqua commented May 24, 2016

spalger commented May 24, 2016

bevacqua commented May 25, 2016

w33ble commented May 26, 2016 •

edited

Loading

bevacqua commented Jun 1, 2016 •

edited

Loading

bevacqua commented Jun 1, 2016

bevacqua commented Jun 1, 2016

w33ble commented Jun 1, 2016

Support configuration reloads for logging #6720

Support configuration reloads for logging #6720

Conversation

bevacqua commented Mar 31, 2016

bevacqua commented Mar 31, 2016

spalger Apr 1, 2016

Choose a reason for hiding this comment

spalger commented Apr 1, 2016

bevacqua commented Apr 1, 2016

spalger commented Apr 1, 2016

spalger commented Apr 5, 2016

bevacqua commented Apr 5, 2016

spalger Apr 5, 2016

Choose a reason for hiding this comment

bevacqua Apr 6, 2016

Choose a reason for hiding this comment

spalger commented Apr 5, 2016

bevacqua commented Apr 6, 2016

bevacqua commented May 24, 2016

spalger commented May 24, 2016

bevacqua commented May 25, 2016

w33ble commented May 26, 2016 • edited Loading

bevacqua commented Jun 1, 2016 • edited Loading

bevacqua commented Jun 1, 2016

bevacqua commented Jun 1, 2016

w33ble commented Jun 1, 2016

w33ble commented May 26, 2016 •

edited

Loading

bevacqua commented Jun 1, 2016 •

edited

Loading